-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update cgat-apps to 0.7.4 #51682
Update cgat-apps to 0.7.4 #51682
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request involves updates to the Possibly related PRs
Warning Rate limit exceeded@mencian has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/cgat-apps/meta.yaml (1)
12-12
: Remove trailing whitespace.There are trailing spaces on line 12 that should be removed.
- +🧰 Tools
🪛 yamllint
[error] 12-12: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- recipes/cgat-apps/meta.yaml (1 hunks)
🧰 Additional context used
🪛 yamllint
recipes/cgat-apps/meta.yaml
[error] 12-12: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/cgat-apps/meta.yaml (3)
1-3
: LGTM: Version update looks correct.The version bump to 0.7.4 aligns with the PR objectives.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
13-15
: LGTM: Build configuration is appropriate.The build number reset to 0 is correct for a new version, and the Python 3.12 skip condition remains appropriate.
8-12
: Verify the source package integrity.Let's verify the SHA256 checksum of the source package.
🧰 Tools
🪛 yamllint
[error] 12-12: trailing spaces
(trailing-spaces)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/cgat-apps/meta.yaml (1)
11-11
: Remove trailing whitespace.There are trailing spaces on line 11.
- sha256: 80a602090d995f6e3214772786b13a34354601c1b4387cfef0aabe9a6323cdbd + sha256: 80a602090d995f6e3214772786b13a34354601c1b4387cfef0aabe9a6323cdbd🧰 Tools
🪛 yamllint
[error] 11-11: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- recipes/cgat-apps/build.sh (1 hunks)
- recipes/cgat-apps/build_failure.linux-64.yaml (0 hunks)
- recipes/cgat-apps/build_failure.osx-64.yaml (0 hunks)
- recipes/cgat-apps/meta.yaml (3 hunks)
💤 Files with no reviewable changes (2)
- recipes/cgat-apps/build_failure.linux-64.yaml
- recipes/cgat-apps/build_failure.osx-64.yaml
🧰 Additional context used
🪛 yamllint
recipes/cgat-apps/meta.yaml
[error] 11-11: trailing spaces
(trailing-spaces)
🔇 Additional comments (8)
recipes/cgat-apps/build.sh (4)
16-16
: LGTM! Modern pip installation approachThe switch to pip installation with appropriate flags (
--no-deps
,--no-build-isolation
) aligns with best practices for conda builds. The verbose output (-vvv
) will help with debugging build issues.
1-16
: Verify build pattern consistency with similar packagesLet's ensure this build pattern aligns with other similar bioinformatics packages in bioconda.
#!/bin/bash # Find similar Python-based bioinformatics packages and compare build patterns echo "=== Packages using pip install ===" rg -l '\$PYTHON -m pip install \.' recipes/ echo -e "\n=== Packages with similar compiler flags ===" rg -l 'export C(FLAGS|PPFLAGS).*-O[23].*PREFIX' recipes/
4-8
: Review compiler optimization settingsThe
-O3
optimization flag is quite aggressive and might affect numerical precision in scientific computing applications. Additionally,-L${PREFIX}/lib
is redundantly specified in bothLDFLAGS
andCFLAGS
.Let's check if other bioconda scientific packages use similar optimization levels:
#!/bin/bash # Search for compiler optimization flags in other scientific package build scripts rg -l 'export C(FLAGS|PPFLAGS).*-O[23]' recipes/
11-13
: Verify setup.py modifications are still validThe sed patterns are quite specific to the package's setup.py structure. Let's verify they still match the current version's setup.py content.
recipes/cgat-apps/meta.yaml (4)
13-13
: LGTM! Build number correctly reset.The build number has been properly reset to 0 for the new version.
64-70
: LGTM! Metadata updates are complete and consistent.The about section has been properly updated with:
- Correct GitHub repository URL
- Complete license information
- Consistent documentation and development URLs
25-26
: Verify pip requirement and cython version constraint.The addition of pip and version constraint for cython aligns with modern Python packaging practices. Let's verify if these changes are consistent with the build script changes.
#!/bin/bash # Check if build.sh uses pip for installation rg -l "pip" recipes/cgat-apps/build.sh # Check for any cython version requirements in setup.py or pyproject.toml fd -t f "setup.py|pyproject.toml" recipes/cgat-apps -x rg -l "cython"
2-2
: Verify SHA256 checksum for the new version.Let's verify the integrity of the source package.
Also applies to: 10-11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
recipes/cgat-apps/meta.yaml (1)
13-13
: Remove trailing whitespaceThere are trailing spaces on this line that should be removed.
- patches: + patches:🧰 Tools
🪛 yamllint
[error] 13-13: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipes/cgat-apps/0001-setup.py.patch
(1 hunks)recipes/cgat-apps/meta.yaml
(3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/cgat-apps/meta.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/cgat-apps/meta.yaml (5)
15-15
: LGTM! Build number correctly reset
The build number has been properly reset to 0 for the new version, following conda-forge best practices.
66-66
: LGTM! Improved package metadata
The updates to the about
section enhance the package metadata with better documentation links and license information.
Also applies to: 68-68, 70-72
27-28
: Verify Cython version compatibility
The addition of pip
and the Cython version constraint change are notable changes. Let's verify the Cython version requirement:
Line range hint 1-72
: Verify completeness of version bump changes
The version bump includes appropriate updates to version, source, build number, and dependencies. However, let's verify if any additional changes are needed:
🧰 Tools
🪛 yamllint
[error] 13-13: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
2-2
: Verify source integrity and patch file
The version update and source URL changes look correct, but let's verify:
Also applies to: 9-12
name='cgat', | ||
version=version, | ||
description='cgat : the Computational Genomics Analysis Toolkit', | ||
@@ -306,22 +137,26 @@ setup( | ||
platforms=["any"], | ||
keywords="computational genomics", | ||
long_description='cgat : the Computational Genomics Analysis Toolkit', | ||
- classifiers=[_f for _f in classifiers.split("\n") if _f], | ||
+ classifiers=[_f for _f in """ | ||
+ Development Status :: 3 - Alpha | ||
+ Intended Audience :: Science/Research | ||
+ Intended Audience :: Developers | ||
+ License :: OSI Approved | ||
+ Programming Language :: Python | ||
+ Topic :: Software Development | ||
+ Topic :: Scientific/Engineering | ||
+ Operating System :: POSIX | ||
+ Operating System :: Unix | ||
+ Operating System :: MacOS | ||
+ """.splitlines() if _f], | ||
url="http://www.cgat.org/cgat/Tools/", | ||
- # package contents | ||
+ python_requires=">=3.6", | ||
packages=cgat_packages, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specify 'install_requires' to ensure dependencies are installed
The install_requires
parameter in the setup()
function is missing. Without it, required dependencies won't be automatically installed when installing the package, leading to potential runtime errors.
Add the install_requires
parameter with the necessary dependencies:
setup(
name='cgat',
version=version,
description='cgat : the Computational Genomics Analysis Toolkit',
...
python_requires=">=3.6",
+ install_requires=[
+ 'numpy',
+ 'Cython>=0.29.35',
+ 'pysam',
+ # Add other dependencies as needed
+ ],
packages=cgat_packages,
package_dir=cgat_package_dirs,
include_package_data=True,
)
Committable suggestion skipped: line range outside the PR's diff.
pysam_libraries = pysam.get_libraries() | ||
-pysam_libdirs = list(set(os.path.dirname(x) for x in | ||
- pysam_libraries)) + conda_libdirs | ||
- | ||
-# remove lib and .so and add htslib | ||
-pysam_libs = ["hts"] + list([os.path.basename(x)[3:-3] for x in | ||
- pysam_libraries]) | ||
- | ||
+pysam_libdirs = list(set(os.path.dirname(x) for x in pysam_libraries)) + conda_libdirs | ||
+pysam_libs = ["hts"] + [os.path.basename(x)[3:-3] for x in pysam_libraries] | ||
pysam_dirname = os.path.dirname(pysam.__file__) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing import statement for 'pysam'
The pysam
module is used in the code (e.g., pysam.get_libraries()
at line 226), but it is not imported. This will result in a NameError
when running setup.py
.
Apply this diff to add the missing import at the beginning of the file:
+import pysam
Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipes/cgat-apps/meta.yaml (1)
13-13
: Remove trailing whitespaceThere is trailing whitespace on this line that should be removed.
- patches: + patches:🧰 Tools
🪛 yamllint
[error] 13-13: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipes/cgat-apps/meta.yaml
(3 hunks)
🧰 Additional context used
🪛 yamllint
recipes/cgat-apps/meta.yaml
[error] 13-13: trailing spaces
(trailing-spaces)
🔇 Additional comments (5)
recipes/cgat-apps/meta.yaml (5)
15-15
: LGTM: Build number reset is appropriate
Build number correctly reset to 0 for the new version.
67-73
: Verify documentation and repository links
The about section has been enhanced with additional URLs. Let's verify these links are accessible.
#!/bin/bash
# Check if URLs are accessible
urls=(
"https://github.com/cgat-developers/cgat-apps"
"https://cgat-apps.readthedocs.io/en/latest"
)
for url in "${urls[@]}"; do
code=$(curl -s -o /dev/null -w "%{http_code}" "$url")
if [ "$code" != "200" ]; then
echo "Warning: $url returned status $code"
fi
done
11-12
: Verify patch file existence and purpose
A new patch file has been added. Let's verify its existence and examine its purpose.
#!/bin/bash
# Check if patch file exists and show its contents
if [ -f "recipes/cgat-apps/0001-setup.py.patch" ]; then
cat recipes/cgat-apps/0001-setup.py.patch
else
echo "Patch file not found"
fi
2-2
: Verify version 0.7.4 release exists
The version has been updated to 0.7.4. Let's verify this release exists and matches the provided checksum.
27-28
: Verify Cython compatibility with the new version
The host requirements have been updated to use pip and specify a minimum Cython version. Let's verify this version constraint against the package's setup requirements.
✅ Verification successful
Further verification required for Cython version compatibility
Further verification required to locate Cython version specifications
Cython version update is appropriate
The update to Cython >=0.29.35 is a reasonable version constraint for modern Python packaging. No compatibility issues were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check setup.py for Cython version requirements
rg -A 5 "cython" recipes/cgat-apps/0001-setup.py.patch
# Check if there are any known issues with Cython >=0.29.35
gh api search/issues -f q="repo:cgat-developers/cgat-apps Cython version"
Length of output: 545
Script:
#!/bin/bash
# Search for Cython version constraints in setup.py
rg "cython\s*>=\s*0\.29\.35" recipes/cgat-apps/setup.py
# Search for changes related to Cython in the patch file
rg "cython" recipes/cgat-apps/0001-setup.py.patch
# Use ast-grep to find Cython imports in the codebase
ast-grep --pattern $'import cython' .
Length of output: 524
Script:
#!/bin/bash
# Find all setup and configuration files in the repository
fd -e pysetup -e toml -e cfg -e yaml -e yml -e json -e txt
# Once files are identified, search for Cython version constraints
rg "cython\s*>=\s*0\.29\.35" **/*.{py,pyi,pysetup,setup.py,pyproject.toml,setup.cfg,requirements*.txt}
# Check documentation files for Cython dependency notes
rg "cython" **/README.md **/*.md
Length of output: 361547
Update
cgat-apps
: 0.7.2 → 0.7.4recipes/cgat-apps
(click to view/edit other files)@cgat-developers
This pull request was automatically generated (see docs).